-
Notifications
You must be signed in to change notification settings - Fork 15
Upgrade Framework o1js Version #323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two questions, failing test should be easy to fix, otherwise good job!
| publicOutput, | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
| publicOutput: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you agree that publicOutput?.publicOutput would be the same semantically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would but we're getting {} instead of null or undefined as publicOutput in const publicOutput = await Reflect.apply(simulatedMethod, this, args); thus this approach gives Property 'publicOutput' does not exist on type '{}'. if we don't check explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay got it. Do we have cases where the type is only a single "publicOutput" level so to speak? Because if so, we should probably find out where and make it the same everywhere
packages/api/package.json
Outdated
| "@proto-kit/protocol": "*", | ||
| "@proto-kit/sequencer": "*", | ||
| "o1js": "^1.6.0", | ||
| "o1js": "^2.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current o1js version is 2.10.0, is this a typo or any particular reason you didn't go to 2.10 directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just in case, I added the same version as the updated mina fungible token, but I thought it wouldn't be a problem since it already uses the latest version on first installation anyway. I fixed it.
| executionData | ||
| ); | ||
| return { | ||
| publicOutput: await proveTransaction( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omg I hate that o1js made this the API, so verbose
This PR upgrades the o1js version used in framework to latest
2.10.0